fix(cli): standardize private key inputs and fix 0x prefix parsing error#423
fix(cli): standardize private key inputs and fix 0x prefix parsing error#423
Conversation
This fixes issue #422 by catching whitespace and padding issues, validating length and properly standardizing the 0x prefix to prevent confusing downstream ckb-ccc cryptography errors.
|
✅ Changeset file detected. |
There was a problem hiding this comment.
Pull request overview
This PR addresses CLI private key parsing inconsistencies by normalizing user-provided private keys (handling optional 0x prefix and common copy/paste artifacts) before constructing CCC signers, preventing confusing downstream cryptography errors.
Changes:
- Added
normalizePrivKey()helper to trim/strip quotes, validate hex, and enforce 32-byte length formatting. - Routed all signer construction through
normalizePrivKey()viaCKB.buildSigner(). - Added a patch changeset entry for
@offckb/cli.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/util/validator.ts |
Introduces normalizePrivKey() for canonical private key normalization + validation. |
src/sdk/ckb.ts |
Applies normalization at the signer construction bottleneck (buildSigner). |
.changeset/fix-privkey-prefix.md |
Declares a patch release for the CLI fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (key.startsWith("'") && key.endsWith("'")) { | ||
| key = key.slice(1, -1); | ||
| } | ||
|
|
There was a problem hiding this comment.
After stripping surrounding quotes, the value is not re-trimmed. Inputs like '"0x... "' (common when copying from .env) will retain inner whitespace and fail the hex-only regex. Consider trimming again after quote removal (and/or before validation) so quoted keys with accidental inner spaces normalize successfully.
| // Trim again to normalize whitespace that was inside surrounding quotes | |
| key = key.trim(); |
src/util/validator.ts
Outdated
| // Remove standard 0x prefix if exists manually for normalization | ||
| if (key.startsWith('0x')) { |
There was a problem hiding this comment.
The 0x prefix stripping is case-sensitive. Keys copied from some tools can start with '0X', which will currently fall through and then fail hex validation due to the 'X'. Consider handling both '0x' and '0X' (e.g., case-insensitive prefix check) before the hex-only regex.
| // Remove standard 0x prefix if exists manually for normalization | |
| if (key.startsWith('0x')) { | |
| // Remove standard 0x/0X prefix if it exists manually for normalization | |
| if (/^0x/i.test(key)) { |
src/util/validator.ts
Outdated
| export function normalizePrivKey(privKey: string): string { | ||
| if (!privKey) return privKey; | ||
|
|
There was a problem hiding this comment.
if (!privKey) return privKey; means empty-string input (or a value like '0x' that becomes empty after prefix stripping) won’t produce the intended descriptive validation error, and will likely fall back to the deeper SignerCkbPrivateKey error. To match the stated goal of failing early with a clear message, consider throwing a dedicated "private key is required" / length error instead of returning unchanged for falsy values.
| return versionRegex.test(version); | ||
| } | ||
|
|
||
| export function normalizePrivKey(privKey: string): string { |
There was a problem hiding this comment.
This normalization/validation logic is now on the buildSigner hot-path for multiple CLI commands, but there are no tests covering common accepted inputs (with/without 0x, with quotes/whitespace) and rejection cases (bad hex, wrong length). Adding a small unit test suite for normalizePrivKey would help prevent regressions and ensure the error messages stay user-friendly.
Resolves #422
Problem
In CLI tools (
deploy,transfer, etc.), depending on how the private key was copied/pasted, the 0x prefix length validation would incorrectly flow to@ckb-ccc/core, ultimately throwing a confusing error:Private key must be 32 bytes!.Solution
This PR introduces a universal
normalizePrivKeyhelper inserted tightly at thebuildSignerbottleneck inside the CKB SDK.0x+ 64 hex characters schema required.